Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New manager layout #636

Draft
wants to merge 67 commits into
base: master
Choose a base branch
from
Draft

New manager layout #636

wants to merge 67 commits into from

Conversation

Mottie
Copy link
Member

@Mottie Mottie commented Jan 2, 2019

This PR is still a work in progress! It's not ready to be merged.

TO DO:

Major

  • Injection order (still needs a lot of work)
    • [ ] Implement the actual injection order of the styles (only the UI is wired up right now).
    • [ ] Update injection order values after importing & deleting.
    • [ ] Add (i) (info icon), next to "#" in header, explaining how and why someone would want to manually sort the injection order.
  • User can only create a new UserCSS style. A non-UserCSS style can't be created since the checkbox is missing. Discuss needed UI changes for this to work...
  • Mass deleting styles may need to check animation frames; optimize code, if possible.
  • Restore export all styles (Added to header).
  • Restore update all styles (Added to header).
  • Make entry name full height clickable.
  • Make entry name show ellipsis when too long.
  • Restore entry hover highlight.
  • Fix header vertical centering? (Please double check this...)

Minor

  • Increase tooltip contrast.
  • Fix header backup dropdown positioning.
  • Reduce favicon size.
  • Discuss removing "support" icon.
  • Change home icon to external link icon.
  • Discuss setting favicons to always show & implement caching of favicons to reduce server calls.
  • Discuss changing green round checkbox into something else?
  • Discuss alternatives to applies to column detail container.
  • Make filters awesome (combine them some how, instead of a checkbox + select).
  • Check entry row styling in narrow browsers

Features

  • Add bulk action to reset UserCSS custom settings.

Missing functionality & stuff that needs work:

  • Get injection order to work
    • Everything in the UI is wired up.
    • Sorting of the injected styles is currently done by style ID; this still needs to be changed.
    • Click on the hamburger icon in the sort column to drag & drop the order.
  • Bulk Actions:
    • I was planning on adding a bulk action to reset UserCSS custom settings, but didn't get around to it.
    • Mass deleting styles may need to check animation frames; optimize code, if possible.
  • As I post this, I realized that you can only create a new UserCSS style. You can't create a non-UserCSS style since the checkbox is missing.
  • Styling when the browser width is < 1100px.

Included functionality:

  • Added bulk actions: enable, disable, export, update & delete.
  • Import styles by using the icon in the header. Dropdown may need to be switched to a modal to keep the look consistent.
  • Sort entries:
    • By clicking, or Shift + click for multi-sort.
    • Included sortable version & last update columns.
  • Added new tooltips.

As this is still a work-in-progress, please test and share any feedback.

@narcolepticinsomniac
Copy link
Member

narcolepticinsomniac commented Jan 2, 2019

Overall, I'm a fan of the concept, and you've done a nice job implementing it, but I'll focus on the faults for now.

Bulk options shouldn't be triggered by simply enabling/disabling individual styles. Export and check for updates shouldn't be buried in bulk actions. If we're not showing applies-to strings anymore, which is fine by me, we might wanna consider pulling favicons by default, bc they're more integral w/out strings. Either way, the icon shown for missing favicons is too pronounced to repeat that often. Probably better off making a real subtle PNG. They're also too big, and should be the standard 16px. The default only showing 3 was designed for strings, and there's no reason to restrict icons until they don't fit in the allotted space.

Not sure we should go that dark on the header, or that subtle on its icons. Style name clickable area should be the entire height of entry, and there's really no need for superfluous edit icons. I prefer external links over "home" icons, which seem overly intricate to be shown repeatedly. If we're leaving icons small, I prefer the previous trashcans. Usercss icons in actions should go to the far left, so they don't leave gaps when they're often absent. It also bugs me that "checked" icons are larger than their empty circle counterparts, and I'm not sure I'm crazy about all the green. There's a few other icons I'd like to nitpick or try alternatives for, but those are the main issues at this stage.

Drag & drop highlight when dragging is kinda pinkish, making it seem like you're doing something wrong. Also, I don't think its functionality will be immediately obvious. There's just a tooltip saying "change style injection order", but when you see the drag & drop cursor, the natural assumption is that you can change the order of the list manually, which of course it doesn't.

Not sure if it's still a WIP, but drag & drop doesn't seem to work at all like I'd expect. If I have it A-Z and drag one style, the whole injection order changes from actual install order to A-Z. Same thing in reverse for Z-A, and I imagine any other current filter ordering. Injection order should only shift on styles with a lower injection number, which should shift down one spot accordingly. Orders seem to be changing drastically based on which filter you're currently sorting by when dragging.

I think drag & drop should only work when sorting by injection order. Its functionality definitely needs to be more obvious, maybe a "don't show this message again" type of popup explaining that you need to filter by injection order to be able to change injection order, and a little explanation of what changing it accomplishes. I think the natural assumption is "higher has priority", when it's the opposite.

I was kinda looking forward to see what you came up with for the new filter UI implementation, but they're the same awkward checkbox/select. NBD, but you'd mentioned alternatives previously, so I was kinda surprised. I'd prefer something a little slicker than details to reveal hidden applies-to rules. IMO, we should replace the ones we have as we update the UI until they're all gone.

@Mottie
Copy link
Member Author

Mottie commented Jan 3, 2019

Bulk options shouldn't be triggered by simply enabling/disabling individual styles.

  • [x ] Fixed

consider pulling favicons by default

I thought about it, but that's a lot of network activity. I thought about caching the icons in localStorage to reduce this issue, but I stopped short when I determined that I'd have to convert each image into a base64 encoded version and store it that way.

icon shown for missing favicons is too pronounced... making a real subtle PNG... default only showing 3

You're right. The icon size is set to be the same as all the other icons. There is actually a css variable (--icon-size) in the manage.css file to set all icon sizes. We could increase this to 6, but more than that it would end up wrapping to the next line and making each entry much bigger.

  • [x ] Increased default to 6.

Not sure we should go that dark on the header, or that subtle on its icons

Yeah, it's still a work in progress. I wanted some more contrast on the manage page, but I guess #111 is too dark. Any suggestions?...

  • [x ] Change header bg color. Edit: I set it to #333

prefer external links over "home" icons ... previous trashcans... actions should go to the far left...

Yeah, I think we could move the home & support external links to the far right; on the other side of the update icon, then we could leave the UserCSS icon where it is... When you say you prefer external links over home icons, do you mean change the icon? What about the support icon? It's also an external link.

  • [x ] Moved the home & support to the far right.

"checked" icons are larger than their empty circle counterparts, and I'm not sure I'm crazy about all the green

Yeah, I'll work on adjusting the sizes. The green is set as a background color, so it would be easy to change. I think I might make it a css variable as well. What default color do you suggest?

  • [x ] Fix checked icon size
  • [x ] Change color of checked icon. *Edit set the default color to hsla(180, 100%, 20%, 1) (set by a css variable).

Drag & drop...

  • [x ] Change row dragging color. Edit now a bluish gradient.
  • [x ] Hide the sort column; only show when # (injection order column) is sorted.
  • [ ] Add (i) (info icon) explaining the above.
  • [ ] Implement the actual injection order of the styles (only the UI is wired up right now).

Filter checkbox + select

Yeah, I thought about changing it but it is really complex with all the different css filters being used to filter the entry rows. And I've been so busy I haven't had time to think of an alternate solution.

  • [ ] Make filters awesome.

Applies to detail

I thought about making it open a modal with the additional favicons, but I really opted for the simplest solution. Using a detail container keeps the accessibility and doesn't look too awful (at least to me).

  • [ ] Make applies to awesome.

@narcolepticinsomniac
Copy link
Member

narcolepticinsomniac commented Jan 4, 2019

I stopped short when I determined that I'd have to convert each image into a base64 encoded version and store it that way

There's gotta be a reasonable way to do it. How does TM handle it? We can keep the ability to disable it, but if we're only showing icons, they shouldn't be the same placeholder repeated endlessly. They're also still 20px which is too big.

Add (i) (info icon) explaining the above.

Where exactly?

Change color of checked icon. *Edit set the default color to hsla(180, 100%, 20%, 1) (set by a css variable).

Much better. Maybe a bit lighter, like hsl(180, 100%, 24%).

Hide the sort column; only show when # (injection order column) is sorted.

On the right track, but it still doesn't work like I'd expect. When filtering by order, styles should be shown in order, but they're not initially, so when you change one, you're still changing the whole list drastically. You'd have to import fresh to see it, because after the initial drastic shift it behaves as expected, but that should never happen. I'm not sure you should even be able to reverse order in that mode.

Change header bg color. Edit: I set it to #333

Header is looking better. Tooltips could use some more contrast now, and the import tooltip overlaps the dropdown.

When you say you prefer external links over home icons, do you mean change the icon?

Yes. A bunch of little houses look silly to me.

What about the support icon? It's also an external link.

I don't even get why they're a thing.

Home is: https://github.com/StylishThemes/Wikipedia-Dark

and help is: https://github.com/StylishThemes/Wikipedia-Dark/issues

Seems redundant. If you like em, it's NBD. As far as them being external, there's no indication, which is whatever. Our "homepage" icon has always been the external, so it's not gonna confuse anyone.

Export and check for updates shouldn't be buried in bulk actions

Still buried.

Style name clickable area should be the entire height of entry

Still not the case.

A couple other things I noticed. What happened to entry hover bgs? Kinda important in table-like layouts. We've also lost toph's hyphen js, so strings w/out spaces don't break. The available space is pretty long and the text is quite a bit smaller, so I think no-wrap ellipsis would be fine. I'm a fan of symmetry, so I like them all being the same height anyway.

A lot of the flex vertical centering issues are simplified by specifying heights when you know what they should always be. Current filter header is too low bc of font-size/positioning of caret. align-self tends to be useless, except for very specific scenarios.

Tried to point them all out in the same screenshot:

2019-01-04 17_33_47-stylus - cent browser

@Mottie
Copy link
Member Author

Mottie commented Jan 13, 2019

I've added a to-do list of things that have been done and still need to be addressed and/or discussed to the first post to keep everything in one place...

@narcolepticinsomniac
Copy link
Member

User can only create a new UserCSS style

Yes there needs to be a toggle, but it defaults to whatever pref is set. Default pref is non-usercss, which is what I see.

only the UI is wired up right now

When I initially filter by order, it still doesn't sort them in chronological order. We can't reorder a list that isn't in order to begin with:

order

Reduce favicon size

You went from too big, to too small. Favicons should be 16px. Globe icons shouldn't have pointer cursor either.

Check entry row styling in narrow browsers

Just undock your browser. It's not great curently, but with a few tweaks it'd be alright.

Fix header vertical centering

Nope. This alignment really bugs me:

align

In this case, it's the caret positioning and font-size. A lot of other flex alignment issues where they get off by a pixel or two usually happen when a parent element wraps elements of slightly different heights. Simplest solution is never align parent elements when avoidable. Specify the height (100% if its parent is specified) and align the child elements.

Discuss removing "support" icon.

I think they can go, but I don't really care. Your call.

Discuss setting favicons to always show & implement caching of favicons to reduce server calls.

Unless there's some terrible performance impact which can't be accounted for, I think we should show them by default.

Discuss changing green round checkbox into something else?

I have no issue with them after you fixed the size and color. Not to say you couldn't try something different if you have any bright ideas.

Discuss alternatives to applies to column detail container.

I just dislike the detail look and feel. Is it just me? As far as alternatives, it'd be something like any other way of revealing an element on click where you could add a snazzy little transition. Icon would be a down arrow which flips to an up arrow when open. Really NBD, I guess, just a preference.

Minor details:

Header icons could use a little space between them. Also kinda difficult to make out the arrows in import/export icons.

Style name hover shouldn't be gray.

Use inline-flex instead of inline-block.

If I didn't mention it, it LGTM. I agree with everything left on the to-do list.

@Mottie
Copy link
Member Author

Mottie commented Jan 28, 2019

I didn't get to everything, but I did:

  • Remove all injection ordering code/UI (for now).
  • Fix favicon size (now a separate css variable).
  • I think I fixed header vertical centering... I wasn't seeing the same misalignment that the screenshot was showing... so I'm not sure if it's completely fixed.
  • Favicons enabled by default.
  • Removed detail/summary applies to; now the extra favicons are just hidden.
  • Increased header icon spacing.
  • Fix entry name hover color.

Stuff I didn't get to yet, but I plan on doing it/something:

  • Add a toggle somewhere for creating a new usercss vs regular style.
  • I forgot about using inline-flex. I'll see if it makes a difference if the alignment is still off in the header.
  • Change home icon to external link icon.
  • Remove support icon.
  • Fix entry row styling in narrow browsers. I know I can use the device toolbar in chrome dev tools; I just haven't put any time into it.
  • Do something with the filters? Still need to think about a nice UI solution.

@Mottie
Copy link
Member Author

Mottie commented Mar 11, 2019

Updated:

  • Create new style button in the header is now a dropdown.
  • Fixed layout for smaller screens.
  • Replaced home icon with an external link icon.

I left the support icon in place because I have a feeling that some users may not associate the home page with reporting issues... maybe we should link them to discord instead?

I'm still not sure what to do with the checkbox + filter dropdown - I'm starting to lean toward making the filter have 3 options: not applied, then the two states. I don't think this should hold up merging this PR.

@Mottie Mottie mentioned this pull request Jul 15, 2019
@eight04 eight04 mentioned this pull request Jan 24, 2020
@Mottie
Copy link
Member Author

Mottie commented Mar 23, 2020

I spent some time updating this branch this weekend (it only took a year 😿).

The last change I made was switching the filter toggle/selects for buttons that add GitHub-like text to the search input. It's not 100% working:

  • One part that still needs fixing is the reset button. I need to figure out why it needs to be pressed twice to reset the filters and show the entire list of entries.
  • Another is making sure that the "Updates or issues" filter button that shows up after updating all styles can be toggled; and hidden after a reset.

I pushed it out to the branch so I could get some feedback on the filter process; and maybe some help troubleshooting the above problems. And it's also nice to have it working properly again, so I can do bulk actions :P

@narcolepticinsomniac
Copy link
Member

narcolepticinsomniac commented Mar 28, 2020

Even just trying to load this branch to test was a nightmare. Man, do I regret testing this branch in my regular profile.

I guess the Chromium key is not set, which I didn't realize until it was too late. My dumb ass tried to set it after the fact. Long story short, I managed to bork Stylus completely in my profile. I couldn't even uninstall and load the master fresh. Every time I restarted my browser it disappeared. Ended up having to revert to an older backup of my portable browser, and swap a bunch of updated configurations and DBs.

I like the general layout concept, as I always did, but what little testing I did get to when loading this branch from scratch, seems like it's seriously buggy. Importing a good local DB was about ten seconds of ridiculous CPU. Once imported, all import/export functionality doesn't seem to work at all. Export-all in the header exports nothing. Select-all-batch exports some randomly low size, like 10% of the actual DB size. I didn't test much beyond import/export.

I wouldn't mind pitching in on this, particularly the responsive design aspect, which is practically non-existent. It needs to be compatible with the master, and import/export needs to work as expected before bothering with anything else IMO.

@Procyon-b

This comment has been minimized.

@Procyon-b
Copy link

@Mottie : I was curious to test.
If I may report a bug.
On the second button bar I can select to display either "usercss" - "non-usercss" and/or "local" - "external" but I can't go back to display everything by un-marking these buttons.

By the look/color of the buttons I would expect the behavior to switch back to the default.

@Mottie
Copy link
Member Author

Mottie commented Apr 6, 2020

By the look/color of the buttons I would expect the behavior to switch back to the default.

Yes, thanks! I spent a lot of time trying to get everything updated and rebased to the master branch, so a bunch of bugs slipped through. I've been really busy with work, so I only get around to working on this feature when I get free time & motivation. @narcolepticinsomniac please feel free to mess around in this branch all you want. And @Procyon-b please feel free to contribute how ever you wish!

@Procyon-b
Copy link

I can't promise I'll contribute code. I've put some of my things on the backburner for a long time, and I would like to get back to them while under lockdown. ;)
I can at least use this layout and report any "annoyance" in the behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants